Feature/new modifier implementation #356
Conversation
github-actions
bot
added
experiment
| caliper_modifier_modes["mode"] = var | ||
| modifier_list.append(caliper_modifier_modes) | ||
| # Add time as the last mode | ||
| modifier_list.append({"name": "caliper", "mode": "time"}) |
There was a problem hiding this comment.
(question) When you mentioned having to order things yourself, is this what you mean?
In a separate conversation, you mention that this is so "In this case, the time mode is always added last to the modifier list which adds the CALI_CONFIG variable after each of the CALI_CONFIG_MODE variables have been defined. "
I'm not clear on how this achieves that. Is time the only modifier mode that defines CALI_CONFIG?
| ) | ||
| elif self.spec.satisfies("caliper=cuda"): | ||
| cuda_support = ( | ||
| self.spec.satisfies("caliper=cuda") and True |
There was a problem hiding this comment.
and True looks vestigial
| # set package spack specs | ||
| package_specs = {} | ||
|
|
||
| if not self.spec.satisfies("caliper=none"): |
There was a problem hiding this comment.
I get the impression that some combinations of caliper=... are not allowed. I don't know if we support conflict yet, but FWIW you can raise an exception here for forbidden combinations.
| from benchpark.experiment import Experiment | ||
|
|
||
|
|
||
| class Caliper(Experiment): |
There was a problem hiding this comment.
(suggestion, not a problem) IIRC you mentioned that caliper would never be instantiated as an experiment itself. FWIW, this could move from var/exp_repo/experiments/caliper/ to lib/benchpark, possibly into a new subdir like lib/benchpark/experiment_addons
| } | ||
| } | ||
|
|
||
| def compute_modifiers_section(self): |
There was a problem hiding this comment.
I have a suggestion for refactoring this to make it easier to add analogs to caliper:
Experiment.__init__(in the base classExperiment) callsExperiment.initialize- Move all functionality in
caliper.Caliperexcept for variant declaration intocaliper.Helper Amg2023.initializesetshelpers = [caliper.Helper()]Experiment(the base class) handles tacking on extra modifiers from all.helpers- It can also take care of
caliper_package_specs
- It can also take care of
The benefit of this organization is that you can add more functionality (that defines other modifiers/specs) without having to modify functions in this class other than .initialize.
New modifier implementation using experiment.py